fix(edit-content): refresh History, Comments and Reference Pages after save #36617 - #36901
Open
adrianjm-dotCMS wants to merge 5 commits into
Open
fix(edit-content): refresh History, Comments and Reference Pages after save #36617#36901adrianjm-dotCMS wants to merge 5 commits into
adrianjm-dotCMS wants to merge 5 commits into
Conversation
…r save #36617 The sidebar's History, Comments and Reference Pages stayed stale after a save/publish, requiring a manual page reload. Two root causes: - The refresh effects lived on DotEditContentSidebarComponent, which the layout's @if destroys and recreates, taking the effects with it. Moved them into store-level withHooks({onInit}) in withActivities and withInformation, matching the existing lock/workflow/history features. - withHistory only invalidated its memo on `${identifier}:${languageId}`, but a save mints a new inode under the SAME identifier and locale, so loadVersions never re-fired. Full-screen rendered an empty list (initializeExistingContent had already cleared it); the dialog host, which never navigates, rendered a stale one. Added two invalidation signals: a cleared list (status INIT, ignored mid-reload so it cannot fetch the identifier being left behind) and a live-inode baseline that only advances when not viewing a historical version, so browsing versions and returning from them does not refetch. Both effects now read the isSidebarOpen leaf instead of uiState(): every writer replaces that slice wholesale, so depending on the object refetched on unrelated UI changes, including the view flip loadVersions performs. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Open
5 tasks
Contributor
|
Claude finished @adrianjm-dotCMS's task in 12m 19s —— View job Code Review — PR #36901
I traced all three fixes against the store's feature composition, the New Issues
Notes (non-blocking)
Overall this is a solid, well-tested fix. No blocking issues. The single Medium is a scoped-edge question, not a regression in the happy path the PR targets. · |
…+ e2e #36617 The feature specs verify the store effects against a synthetic store with no component mounted, so they cannot catch the regression this fix addresses: the effects used to live on DotEditContentSidebarComponent, which the layout's @if destroys and recreates. Moving them back would leave every feature spec green. Two layers close that gap, both parameterized over the full-screen and dialog hosts since the bug had a different face in each: - Integration (dot-edit-content.layout.component.spec.ts): real store mounted in the real layout with the sidebar as a MockComponent. Asserts the fetches happen while the sidebar is not rendered at all, that they survive the sidebar being destroyed and recreated, and that a save minting a new inode refreshes without any re-initialization (the dialog path). Verified these fail when the store hooks are removed. - E2E (apps/dotcms-ui-e2e/.../sidebar/history-refresh.spec.ts): publishes and comments through the UI and asserts History/Comments update with no reload, in full-screen and in the dialog (reached via the relationship field's "New Content", which needs no page/template fixture). Verified against the pre-fix code: full-screen reproduced the empty list (expected 2, got 0) and the dialog reproduced the stale list (expected 2, got 1). Dialog comments are deliberately not covered — the comment form is hidden for content opened as 'new', which is the only mode that entry point offers. It needs the UVE pencil flow and a page fixture; documented in the spec. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…not-refreshing-when
`nx format:check` — the CI's format-test goal — flagged this file. The lint-staged hook ran `nx format:write` on it at commit time but left it unformatted, so the check only surfaced in the pipeline. Formatting only; no test logic changed. 2186 tests still pass. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
adrianjm-dotCMS
marked this pull request as ready for review
August 6, 2026 13:35
adrianjm-dotCMS
requested review from
KevinDavilaDotCMS,
dario-daza,
nicobytes,
oidacra,
rjvelazco and
zJaaal
August 6, 2026 14:03
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #36617
Proposed Changes
The sidebar's History, Comments and Reference Pages stayed stale after a save/publish and needed a manual page reload. There were two independent root causes.
The refresh effects lived on a component that gets destroyed. They sat on
DotEditContentSidebarComponent, which the layout's@ifdestroys and recreates — taking the effects with it. Moved into store-levelwithHooks({ onInit })inwithActivitiesandwithInformation, so they live as long as the store does. This matches the existinglock/workflow/historyfeatures.withHistorydid not recognise a newly minted version. Its memo only invalidated on${identifier}:${languageId}, but a save mints a new inode under the same identifier and locale, soloadVersionsnever re-fired. The symptom differed per host, which is why it looked like two separate bugs:Screen.Recording.2026-08-05.at.3.47.52.PM.mov
initializeExistingContentversions: [])Added two invalidation signals: a cleared list (
status === INIT, ignored mid-reload so it cannot fetch the identifier being left behind) and a live-inode baseline that only advances when not viewing a historical version, so browsing versions — and returning from them — does not refetch.store.uiState(), i.e. the whole slice. Every writer replaces that object wholesale, so the effects refetched on unrelated UI changes — including theviewfliploadVersionsperforms internally, costing 3–4 redundant requests per save. Now they read thestore.uiState.isSidebarOpen()leaf.Checklist
Additional Info
Tests: 2180 passing across 111 suites in
edit-content; lint and typecheck clean. New coverage includes publish-in-dialog, publish-while-comparing, the mid-reload case, historical/compare round trips, and a regression test for the signal-granularity fix (verified it fails against the previous code).One existing test changed expectation —
should not reload anything when only the version inode changesasserted that moving only the inode does not reload, which was literally the bug. Rewritten asshould reload versions but not push publish history when the live inode moves. This is safe because browsing versions always goes throughloadVersionContent, which setsisViewingHistoricalVersion; a bare inode move only happens on save/publish. The browsing cases are now covered by dedicated tests that set the realistic state.Known follow-ups, deliberately out of scope:
clearVersions()/resetVersions()/clearPushPublishHistory()/resetPushPublishHistory()inhistory.feature.tshave no production call sites — dead code worth removing separately.🤖 Generated with Claude Code